no need to typedef this enum

Removed typedef as suggested.

no need to typedef this enum

Removed typedef as suggested.

I'm seeing a pattern, but yeah like the others, don't typedef enums/structs please. We somewhat follow linux coding style and typedefs like this aren't warranted.

https://kernel.org/doc/html/latest/process/coding-style.html#typedefs

Thanks for the reference. Removed typedef as suggested.

use the WAIT_FOR macro here with a time limit if the hardware has some guarantees on time to reset you can follow and fail on

Used WAIT_FOR macro with timeout

not needed if you use goto and labels or early returns, please fix and drop do { } while(0);

We follow MISRA C rule 15.5, which recommends having a single point of exit at the end of a function. While there are multiple ways to achieve this, we generally avoid using goto, as it’s often discouraged. Using the do { } while (0) construct is one valid way to meet this rule.
What’s your opinion on this approach within Zephyr’s coding style?

@teburd We have already discussed the use of do { } while(0) in another PR: #93450 (comment) , which has now been approved and merged.

MISRA C:2012 Rule 15.1 – The goto statement should not be used:
https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_15_01.c

MISRA C:2012 Rule 15.5 – A function should have a single point of exit at the end:
https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/R_15_05.c

We aim to comply with both rules wherever possible, which is why we use the do { } while(0) style here.

Zephyr is massively not compliant then already as almost everything uses goto, including the kernel in large numbers of functions.

do {} while(0) is a relatively new approach to this in the zephyr codebase, and not one that seems to add readability in my view, @nashif @kartben do we have any guidelines on these misra rules? I'd argue readability matters a great deal more and this is less readable to me at least than the much more commonly practiced style that we have throughout Zephyr.

if (some_err_branch_check) {
   ret = -EIO;
   goto out;
}

// more code

out
   return ret;

As far as I can see the rule 15.1 is "unrestricted use of goto is bad" but I don't know for certain as the rules are not published publicly.

But Misra has a few rules around usage of goto according to mathworks.

https://www.mathworks.com/help/bugfinder/ref/misrac2023rule15.2.html
https://www.mathworks.com/help/bugfinder/ref/misrac2023rule15.3.html

Why have any rules around using goto if you can't use it, makes no sense.

I understand that the intention here is to avoid multiple exit point from a function (https://in.mathworks.com/help/bugfinder/ref/misrac2023rule15.5.html). This is an advisory Misra C rule. To avoid multiple returns, there are multiple methods.
Some are

1. Restructure the logic
2. encapsulate inside do-while(0) with break statement
3. use goto statement.

First preference should be option 1. Second option can be option 2, do-while(0). The usage of goto is not recommended as it violates another advisory Misra C rule(https://in.mathworks.com/help/bugfinder/ref/misrac2023rule15.1.html) . Usage of goto is like violating one advisory rule to fix another advisory rule.
So I would suggest to live with multiple returns whenever do-while(0) doesn't solve the multiple return issue.

@ArunMCHP, in this particular case, i could see that the issue can be solved by option 1, by restructuring the code without using do-while. Could you please explore that option?

@teburd , the Misra rules you mentioned (15.2 and 15.3) are required rules.
I hope the zephyr will not prevent anyone from following Misra rules as long as it doesn't violates any of the zephyr coding guidelines.

No one is preventing anyone from following the MISRA rules. As far as I know 15.1 is not a required rule.

That said using a different style of single exit than the rest of the code base absolutely stood out to me and took a second to read. Readability matters a lot I'd say in a group project like this. To me this is less readable, but if the community feels this is a good approach I'm good with it! So I'd say this is worth discussing in one of the architecture meetings if that makes sense.

@carlescufi can we add this topic, specifically around the usage of do { } while(0) in place of goto for single exit?

k_fifo really isn't the right tool for this, either use a k_mem_slab, sys_block, or array with atomic bitmap signaling used/unused. If you want the cheapest computational route make an array of arrays, one array for each channel with some number of descriptors. No locks, no lists, no checks needed at all since channels are single owner.

This undef looks suspicious, if nothing else its going to lead to confusion if someone greps around for DMAC_REGS and sees there's two defines for it. If the token is already defined then use a new one, no need to undef and redef.

Used a new macro.

extra unnecessary spacing here

Removed extra line.

Really a goto here is just fine, we use them everywhere in Zephyr for single exits, you can drop the irq_locked flag as well and have two exit labels.

Replaced the do { } while (0) single-exit construct with if-else based flow for better clarity wherever possible.
Also removed irq_lock() since no shared resources are accessed among channels.

irq_lock still here it appears, if the register that is being manipulated isn't shared (it doesn't appear to be) among channels, then it is safe to do so without locking.